Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace execution context controls with record mode controls #9133

Merged
merged 12 commits into from
Feb 23, 2024

Conversation

kazcw
Copy link
Contributor

@kazcw kazcw commented Feb 21, 2024

Pull Request Description

Closes #8673. Stacked on #9132.

Before:
image
image

After:
image
image

Important Notes

  • The model has been refactored from execution-modes (live/design) to record-mode (on/off) up to the point of the project store, where it's translated (since the backend still uses the live/design concepts).

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • The documentation has been updated, if necessary.
  • Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
  • All code follows the
    Scala,
    Java,
    and
    Rust
    style guides. In case you are using a language not listed above, follow the Rust style guide.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed, the GUI was tested when built using ./run ide build.

@kazcw kazcw added the -gui label Feb 21, 2024
@kazcw kazcw self-assigned this Feb 21, 2024
@kazcw kazcw added the CI: No changelog needed Do not require a changelog entry for this PR. label Feb 21, 2024
Copy link
Contributor

@farmaazon farmaazon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM except the changes from the previous PR (see my comment in #9132 )

Comment on lines 653 to 661
<symbol id="record" viewBox="0 0 16 16" width="16" height="16" fill="none">
<circle cx="8" cy="8" r="7" fill="currentColor" />
</symbol>

<symbol id="record_once" viewBox="0 0 24 16" width="24" height="16" fill="none">
<rect x="0" y="1" width="3" height="14" rx="1" fill="currentColor" />
<circle cx="12" cy="8" r="7" fill="currentColor" />
<rect x="21" y="1" width="3" height="14" rx="1" fill="currentColor" />
</symbol>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were they added to figma project? If not, then they will be removed upon next export.

@AdRiley
Copy link
Member

AdRiley commented Feb 23, 2024

Can you share a screenshot of what was previously the design/live mode toggle in the on/off positions?

@kazcw
Copy link
Contributor Author

kazcw commented Feb 23, 2024

Can you share a screenshot of what was previously the design/live mode toggle in the on/off positions?
@AdRiley

Added to description. (The padding is a bit ugly in the Before pics because it's an interim state between moving the project name and this PR).

@AdRiley
Copy link
Member

AdRiley commented Feb 23, 2024

Looks good! Thank you :)

@kazcw
Copy link
Contributor Author

kazcw commented Feb 23, 2024

I modified the record-once icon to be 16x16 because we're using exclusively 16x16 icons. This might not be exactly the icon design we want, but I'm going to merge it to get the code changes in; we can tweak the icon later.

@kazcw kazcw added the CI: Ready to merge This PR is eligible for automatic merge label Feb 23, 2024
@mergify mergify bot merged commit 245ec7a into develop Feb 23, 2024
26 of 27 checks passed
@mergify mergify bot deleted the wip/kw/record-control/1-new-ui branch February 23, 2024 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-gui CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace Design/Live/Run control with Record/Record Once Control
3 participants